Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

case-insensitive comparisons in unit testing, base unit testing test #55

Merged
merged 14 commits into from
Feb 9, 2024
Merged

case-insensitive comparisons in unit testing, base unit testing test #55

merged 14 commits into from
Feb 9, 2024

Conversation

MichelleArk
Copy link
Contributor

@MichelleArk MichelleArk commented Jan 29, 2024 •
edited by dbeatty10
Loading

resolves dbt-labs/dbt-core#9467
resolves #84

docs dbt-labs/docs.getdbt.com/

Problem

No base test exists for adapter-specific functionality for the new unit testing framework in dbt-core. This functionality includes pieces like:

  • mocking out inputs with different types
  • case-insensitive 'actual vs expected' query building
  • input validation

Solution

  • Introduce base tests for types & case insensitivity
  • Update unit testing macros to generate a case-insensitive unit test query + introduce a pattern for implementing a common cast macro that safe_cast can leverage as appropriate.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

Note: This repo does not yet run integration tests, but I've tested these in a local venv built from dbt-core but pointing to these changes for postgres. Other 1p adapters inherit the base tests and are run in separate repos

@MichelleArk MichelleArk mentioned this pull request Feb 6, 2024
Closed
5 tasks
@MichelleArk MichelleArk marked this pull request as ready for review February 7, 2024 15:50
@MichelleArk
Copy link
Contributor Author

MichelleArk commented Feb 7, 2024 •
edited
Loading

Snowflake: dbt-labs/dbt-snowflake#896
BigQuery: dbt-labs/dbt-bigquery#1031
Redshift: dbt-labs/dbt-redshift#680
Spark: dbt-labs/dbt-spark#976

gshank
gshank previously approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. Nice error message for invalid column names.

gshank
gshank previously approved these changes Feb 7, 2024
View reviewed changes
mikealfare
mikealfare reviewed Feb 7, 2024
View reviewed changes
Copy link
Contributor

@mikealfare mikealfare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing alarming. I have some cause for concern that we're bypassing quoting and case-sensitivity. But that's literally in the title and seems to be the intention, so I'll chalk that up as me lacking context.

dbt/include/global_project/macros/utils/cast.sql Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_case_insensitivity.py Outdated Show resolved Hide resolved
mikealfare
mikealfare reviewed Feb 8, 2024
View reviewed changes
Copy link
Contributor

@mikealfare mikealfare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the double review. I just missed these I guess.

dbt/tests/adapter/unit_testing/test_case_insensitivity.py Outdated Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_case_insensitivity.py Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_invalid_input.py Outdated Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_invalid_input.py Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_types.py Outdated Show resolved Hide resolved
dbt/tests/adapter/unit_testing/test_types.py Show resolved Hide resolved
dbt/include/global_project/macros/unit_test_sql/get_fixture_sql.sql Show resolved Hide resolved
@MichelleArk MichelleArk force-pushed the unit-testing-case-insensitive-comparisons branch from 82c83b7 to 75254ad Compare February 9, 2024 01:02
@MichelleArk MichelleArk force-pushed the unit-testing-case-insensitive-comparisons branch from 75254ad to f9c1080 Compare February 9, 2024 01:05
@MichelleArk MichelleArk force-pushed the unit-testing-case-insensitive-comparisons branch from f9c1080 to a138e28 Compare February 9, 2024 01:12
@mikealfare mikealfare merged commit c30497e into main Feb 9, 2024
5 checks passed
@mikealfare mikealfare deleted the unit-testing-case-insensitive-comparisons branch February 9, 2024 01:35
@jtcohen6 jtcohen6 mentioned this pull request Feb 19, 2024
Merged
4 tasks
This was referenced Apr 13, 2024
Merged
Merged
mirnawong1 added a commit to dbt-labs/docs.getdbt.com that referenced this pull request Apr 16, 2024
@mirnawong1
Cross-database cast macro (#5278)
bf87975 
[Preview](https://docs-getdbt-com-git-dbeatty10-patch-3-dbt-labs.vercel.app/reference/dbt-jinja-functions/cross-database-macros)

> [!NOTE]  
> I didn't make much of an attempt at versioning this thoughtfully. So
please update this as-needed to bring it in line with expectations.

## What are you changing in this pull request and why?

dbt-labs/dbt-adapters#84 was implemented in
commit
dbt-labs/dbt-adapters@5a50be7
within PR dbt-labs/dbt-adapters#55

So this docs PR adds it to the listing of [cross-database
macros](https://docs.getdbt.com/reference/dbt-jinja-functions/cross-database-macros).

## Checklist
- [x] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
so my content adheres to these guidelines.
@dbeatty10 dbeatty10 mentioned this pull request Apr 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbeatty10 dbeatty10 dbeatty10 left review comments

@mikealfare mikealfare mikealfare approved these changes

@colin-rogers-dbt colin-rogers-dbt Awaiting requested review from colin-rogers-dbt

@gshank gshank Awaiting requested review from gshank

Assignees

@MichelleArk MichelleArk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-1229] [Feature] Cross-database cast macro case insensitive column names for unit test YML
4 participants
@MichelleArk @gshank @mikealfare @dbeatty10